rename config examples + remove roles/*/*-config.toml
#726
Conversation
roles/translator/tproxy-config.toml
Outdated
There was a problem hiding this comment.
every role has a *-config.toml on the crate root
is there any special reason to delete only for the translator?
There was a problem hiding this comment.
I hadn't paid attention to it :)
I don't like having some configs also in the root, we already provide the examples.
But I'm not 100% against them, I don't know.
What do you think about it?
There was a problem hiding this comment.
no strong opinions from my side either, but if I had to take a stance, I would agree they are kind of unnecessary
but if we remove from translator, we should remove from every role to keep the same standards
There was a problem hiding this comment.
no strong opinions from my side either, but if I had to take a stance, I would agree they are kind of unnecessary
but if we remove from translator, we should remove from every role to keep the same standards
Yeah I totally agree with you.
What do you think about this @Fi3 ?
There was a problem hiding this comment.
Is ok to not have them in the crate root. I usally keep them there for convenience and change them very often, so is hard to remeber every time to not commit changed to the config files. Also I noticed that we tend to remove them and then recommit them, if we remove them we should try to remeber it when we do reviews (i'm the first one to forget it). Or maybe just add anc action that fail if the config are there
There was a problem hiding this comment.
Is ok to not have them in the crate root. I usally keep them there for convenience and change them very often, so is hard to remeber every time to not commit changed to the config files. Also I noticed that we tend to remove them and then recommit them, if we remove them we should try to remeber it when we do reviews (i'm the first one to forget it). Or maybe just add anc action that fail if the config are there
We could add them in the gitignore (?)
There was a problem hiding this comment.
ok I removed the files from all roles and added /roles/*/*-config.toml to the root .gitignore
There was a problem hiding this comment.
@plebhash I would edit the values min_individual_miner_hashrate and channel_nominal_hashrate (in both config-example files) with the value 10_000_000_000_000 (which are 10Th/s).
Since the real testing is done with ASIC machines, it makes no sense anymore to use the current tiny values in examples config imo.
In any case, I would remind to have a look at that parameter in the instructions as well.
plebhash
changed the title
roles/*/*-config.toml
ack db91a87 |
|
@plebhash can you just add something like |
plebhash
force-pushed
the
rename-config-examples
branch
from
8af1f5f
to
8346e17
Compare
this topic was already discussed with @Fi3 on a parallel PR that hasn't yet been merged:
#684 (comment)
The naming conventions on
config-examplesofroles/jd-clientandroles/translatorwere a bit confusing/misleading.I'm working to update the tutorials from
Getting Startedsection of stratumprotocol.org and ideally we should fix this in order to avoid confusion on the community (stratum-mining/stratumprotocol.org#190)I'm sending this PR since #684 is low prio and may not be merged soon, while updating the tutorials is more urgent.
additionally, as discussed in the review comments, this PR is also removing
roles/*/*-config.tomlfiles, and adding them to the.gitignoreto avoid getting them back into the commit history